Fix TitleCase not capitalizing the first word when it is an article, conjunction, or preposition (#1658) #1663
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 736cad7eb4
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if(AllCapitals(value)) | ||
| { | ||
| continue; | ||
| } | ||
| if (!isFirstWord && IsArticleOrConjunctionOrPreposition(value)) |
There was a problem hiding this comment.
When the first word is all caps, the continue exits before isFirstWord is flipped, so the next word is still treated as the first word. That means a conjunction/article after an all‑caps first word (e.g., "NASA and the future") will be capitalized as if it were first, which regresses the prior behavior where "and" stayed lowercase. This only happens when the first token is all caps, but it breaks standard title‑case rules for those inputs; consider updating isFirstWord before the continue or otherwise marking the first word as consumed.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR fixes an issue where ToTitleCase failed to capitalize the first word when it was an article, conjunction, or preposition, which violates standard English title-casing rules requiring the first word to always be capitalized.
Key Changes:
- Introduced
isFirstWordtracking in the title case transformation logic to ensure the first word is always capitalized regardless of its part of speech - Split the conditional logic to separately handle all-capitals words and article/conjunction/preposition exceptions
- Added test cases covering articles, conjunctions, and prepositions as first words
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/Humanizer/Transformer/ToTitleCase.cs |
Modified the Transform method to track whether the current word is the first word and only skip article/conjunction/preposition capitalization for non-first words |
src/Humanizer.Tests/TransformersTests.cs |
Added new test method TransformToTitleCase_FirstWordExceptions with test cases for articles, conjunctions, and prepositions as first words |
| var value = word.Value; | ||
| if (AllCapitals(value) || IsArticleOrConjunctionOrPreposition(value)) | ||
|
|
||
| if(AllCapitals(value)) |
There was a problem hiding this comment.
Missing space after 'if' keyword. Should be 'if (' instead of 'if(' to follow the project's consistent spacing convention.
| if(AllCapitals(value)) | |
| if (AllCapitals(value)) |
|
|
||
| if(AllCapitals(value)) | ||
| { | ||
| continue; | ||
| } | ||
| if (!isFirstWord && IsArticleOrConjunctionOrPreposition(value)) | ||
| { | ||
| continue; | ||
| } | ||
|
|
||
| isFirstWord = false; | ||
|
|
There was a problem hiding this comment.
The 'isFirstWord' flag should be set to false after encountering the first word, even if that word is skipped due to being all-capitals. Currently, if the first word is all-caps (like an acronym), the flag remains true, causing the second word to be incorrectly treated as the first word and capitalized even if it's an article, conjunction, or preposition. Move the 'isFirstWord = false' assignment before the conditional checks.
| if(AllCapitals(value)) | |
| { | |
| continue; | |
| } | |
| if (!isFirstWord && IsArticleOrConjunctionOrPreposition(value)) | |
| { | |
| continue; | |
| } | |
| isFirstWord = false; | |
| var wasFirstWord = isFirstWord; | |
| isFirstWord = false; | |
| if(AllCapitals(value)) | |
| { | |
| continue; | |
| } | |
| if (!wasFirstWord && IsArticleOrConjunctionOrPreposition(value)) | |
| { | |
| continue; | |
| } |
| [Theory] | ||
| [InlineData("a great article", "A Great Article")] | ||
| [InlineData("yet another conjunction", "Yet Another Conjunction")] | ||
| [InlineData("by this preposition", "By This Preposition")] | ||
| public void TransformToTitleCase_FirstWordExceptions(string input, string expectedOutput) => | ||
| Assert.Equal(expectedOutput, input.Transform(To.TitleCase)); |
There was a problem hiding this comment.
Consider adding test coverage for the edge case where the first word is all-capitals (like an acronym) followed by an article, conjunction, or preposition. For example, test cases like 'NASA and the mission' or 'API for developers' to ensure the second word is handled correctly based on whether it should be treated as appearing after the first word.
2 participants
This PR fixes an issue where ToTitleCase failed to capitalize the first word when that word was an article, conjunction, or preposition. This resulted in incorrect output. According to standard English title‑casing rules, the first word must always be capitalized, regardless of its part of speech.
Fixes #1658